feat(gemini): add thought signature preservation for thinking models #1622
Conversation
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
pgrayy
force-pushed
the
gemini-thought-signature
branch
from
192e3c7 to
2811c58
Compare
pgrayy
force-pushed
the
gemini-thought-signature
branch
from
2811c58 to
6450f71
Compare
|
/strands review |
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
| current_tool_use["name"] = tool_use_data["name"] | ||
| current_tool_use["input"] = "" | ||
| if "reasoningSignature" in tool_use_data: | ||
| current_tool_use["reasoningSignature"] = tool_use_data["reasoningSignature"] |
There was a problem hiding this comment.
Issue: Missing test coverage for reasoningSignature handling.
Codecov reports 0% patch coverage for the streaming.py changes. The existing parameterized tests for handle_content_block_start and handle_content_block_stop should be extended to cover the reasoningSignature field.
Suggestion: Add test cases to the existing parameterized tests in test_streaming.py:
# In test_handle_content_block_start parameters:
(
{"start": {"toolUse": {"toolUseId": "test", "name": "test", "reasoningSignature": "YWJj"}}},
{"toolUseId": "test", "name": "test", "input": "", "reasoningSignature": "YWJj"},
),
# In test_handle_content_block_stop parameters (with reasoningSignature):
(
{
"content": [],
"current_tool_use": {"toolUseId": "123", "name": "test", "input": '{}', "reasoningSignature": "YWJj"},
"text": "",
"reasoningText": "",
"citationsContent": [],
"redactedContent": b"",
},
{
"content": [{"toolUse": {"toolUseId": "123", "name": "test", "input": {}, "reasoningSignature": "YWJj"}}],
...
},
),
|
Issue: PR description is incomplete. Please update the PR description to include:
This helps reviewers understand the context and purpose of the changes. |
Review SummaryAssessment: Request Changes OverviewThis PR adds thought signature preservation for Gemini thinking models by:
Key ThemesTest Coverage Gap (Important) PR Documentation What Looks Good
Requested Changes
Overall the implementation looks solid - just needs the test coverage addressed before merging. |
3 participants
Description
Related Issues
Documentation PR
Type of Change
Bug fix
New feature
Breaking change
Documentation update
Other (please describe):
Testing
How have you tested the change? Verify that the changes do not break functionality or introduce warnings in consuming repositories: agents-docs, agents-tools, agents-cli
hatch run prepareChecklist
By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.